Add Windows pod sandbox stats information to KEP 2371 - cri pod container stats #3439
Conversation
k8s-ci-robot
added
sig/windows
k8s-ci-robot
added
the
kind/kep
k8s-ci-robot
requested review from
bobbypage,
dcantah,
haircommander and
marosset
k8s-ci-robot
added
size/XXL
jsturtevant
force-pushed
the
add-windows-podstats-details
branch
3 times, most recently
from
acae793 to
1fa4600
Compare
k8s-ci-robot
added
size/L
|
/assign @haircommander @bobbypage @dchen1107 |
|
|
||
| The field `LinuxPodSandboxStats` would stay the same (with duplicated fields) for backwards compatibility. We will also leave `WindowsPodSandboxStats` with a comment that it is not currently used. This option allows us to add specific fields in the future, Linux and Windows sub fields could be added to `LinuxPodSandboxStats` and `WindowsPodSandboxStats` if there are use cases for specific fields. | ||
|
|
||
| The downside to this approach is that we have two fields (`LinuxPodSandboxStats` and `PodSandboxStats`) which are initially identical. We could mark the individual fields in the `LinuxPodSandboxStats` as depreciated and allow only new linux only fields be added to `LinuxPodSandboxStats`. |
There was a problem hiding this comment.
I think this approach makes sense. I think we should mention that eventually we will drop LinuxPodSandboxStats and WindowsPodSandboxStats in favor of the single structure
There was a problem hiding this comment.
sounds good, I was thinking that we could possibly use the fields in the future for OS specific stats but probably better to handle at a later point if it ever comes up in the future.
|
@bobbypage @haircommander @mikebrow I have this on the agenda to discuss tomorrow. I won't be able to make it but @marosset is going to attend. Hopefully we can get some consensus on the direction forward. Thanks! |
Signed-off-by: James Sturtevant <[email protected]>
jsturtevant
force-pushed
the
add-windows-podstats-details
branch
from
2845607 to
73a5c8d
Compare
k8s-ci-robot
removed
the
lgtm
|
After further discussion at sig-node on Sep 13th, it was decided that we will keep Windows and Linux fields separate. I've updated the KEP @haircommander @marosset @bobbypage @dcantah |
|
/milestone v1.26 |
| - balancing_enabled | ||
| - dm_operation_in_progress | ||
|
|
||
| These are not used currently but are be very specific to each implementation and could be collected by specialized tools on a as needed basis. |
There was a problem hiding this comment.
or maybe reported as an unstructured Metric through the endpoint formally known as /metrics/cadvisor?
There was a problem hiding this comment.
had the same thoughts, exposing them through that would be a good way to provide them to end users. If kubelet ends up needing them for decisions, we could introduce them to the formal Pod Stats call but can be done at a later time if need arises
|
/lgtm there's one leading question I added, but I don't think we need to hammer out the details of it yet. edit: oops, missed #3439 (comment), I'm on standby to re-lgtm :) |
k8s-ci-robot
added
the
lgtm
Co-authored-by: Danny Canter <[email protected]>
k8s-ci-robot
removed
the
lgtm
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
After further discussion at sig-node on Sep 13th, it was decided that we will keep Windows and Linux fields separate. The KEP has been updated to reflect this. /assign @dchen1107 @mrunalp @mikebrow |
|
/lgtm |
|
/assign @derekwaynecarr |
|
/lgtm |
|
/cc @mikebrow @derekwaynecarr for 1.26 enhancement freeze |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: derekwaynecarr, jsturtevant The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Signed-off-by: James Sturtevant [email protected]
Adding Windows stats details to the cri pod container stats KEP
After opening kubernetes/kubernetes#110754 we discovered there was more design work need for Windows and was suggested at sig-node to do the work in KEP.
/sig windows
/sig node